Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

improve http request handling for sources and support multiple paths on same netloc #5518

Merged
merged 3 commits into from
May 7, 2022
Merged

improve http request handling for sources and support multiple paths on same netloc #5518

merged 3 commits into from
May 7, 2022

Conversation

abn
Copy link
Member

@abn abn commented Apr 29, 2022 •
edited
Loading

This change refactors HTTP repository source implementations. The following changes have been made.

  • CacheControl cache now lives within Authenticator.
  • Authenticator manages unique sessions for individual netloc.
  • CacheControl usage now respects disable cache parameter in repos.
  • Certificate and authentication logic is now managed solely within
      Authenticator for source repositories taking advantage of recent
      enhancements.
  • Support same netloc with multiple path's in Authenticator.
  • Clean up and unifiy redundant logic in credential access.

These changes should allow for better handling of cases like those described in #3041. Additionally, this forms the foundation for unifying HTTP specific logic within the code base and possibly allowing for migration of requests etc. if/when required.

Additionally, also cleans up some type hinting issues pertaining to http repo only attributes.

Co-Authors: @Darsstar @agni-sairent

Closes: #5108
Closes: #5463
Resolves: #5651

@abn abn requested a review from a team April 29, 2022 13:17
This was referenced Apr 29, 2022
Closed
Merged
@Darsstar
Copy link
Contributor

Darsstar commented Apr 29, 2022 •
edited
Loading

Azure DevOps Aftifacts Feed + keyring still works 😄

I'm just going to mention #5463 and tag @agni-sairent so that both of you know about each other's PR.

@abn
Copy link
Member Author

abn commented Apr 29, 2022

@Darsstar 🍰 Thank you for testing that much appreciated and for the #5463.

@agni-sairent would you mind if I pulled in your changes into this PR so its all tidy? I am guessing the use case things like gitlab etc where there are project specific credentials?

@agni-sairent
Copy link
Contributor

@abn go for it! I don't mind at all 😄
You're exactly right, our use case is multiple gitlab repos/package registries with different credentials.

@abn
Copy link
Member Author

abn commented Apr 29, 2022 •
edited
Loading

@Darsstar @agni-sairent I have landed that change at abn#1 for now - might need a couple of iterations. Feedback/testing appreciated. I just reused tests from #5463 - well, whatever was net new.

This PR can go ahead without those changes as this does not change existing functionality.

@KundaPanda
Copy link

KundaPanda commented May 4, 2022 •
edited
Loading

I am looking forward to seeing either one of these PRs merged, so I went forward and tried your version but discovered some odd behaviour.

  • If I specify a custom source, which only contains specific packages, and I try to install another package from pypi (in my case, pytz), poetry cannot resolve it. (Because test depends on pytz (^2022.1), which doesn't exist, version solving failed.)

    • This issue disappears once I have removed the source from pyproject.toml. After adding custom source(s) back, the issue arises again.
    • All custom repositories are set as secondary, and adding an official pypi source as default did not help either.
    • Oddly enough, I have only encountered this issue with pytz, which I am able to install as whl, so it might be unrelated to this PR and only specific (maybe?) to year-based version names.
    • This happens on Issue #5107: Support multiple respositories on same host with different credentials. #5463 as well.

  • Custom packages need to be specified with source; otherwise, poetry only looks on pypi.org and fails with 404.

EDIT: First issue happens on master as well, second already described in #5476

This was referenced May 5, 2022
Closed
Closed
Open
@abn abn changed the title improve http request handling for sources improve http request handling for sources and support multiple paths on same netloc May 5, 2022
Darsstar
Darsstar suggested changes May 6, 2022
View reviewed changes
Copy link
Contributor

@Darsstar Darsstar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there! Lots of nice refactoring!

src/poetry/utils/authenticator.py Outdated Show resolved Hide resolved
src/poetry/utils/authenticator.py Outdated Show resolved Hide resolved
tests/utils/test_authenticator.py Outdated Show resolved Hide resolved
@abn abn force-pushed the http-repo-cleanup branch 2 times, most recently from 4f233a4 to 2e0664b Compare May 6, 2022 10:58
Darsstar
Darsstar previously approved these changes May 6, 2022
View reviewed changes
abn and others added 3 commits May 7, 2022 17:41
@abn
pip installer: fix typing issues for http repos
b80d8ca
@abn
improve http request handling for sources
5f57602 
This change refactors HTTP repository source implementations. The
following changes have been made.

- CacheControl cache now lives within Authenticator.
- Authenticator manages unique sessions for individual netloc.
- CacheControl usage now respects disable cache parameter in repos.
- Certificate and authentication logic is now managed solely within
  Authenticator for source repositories taking advantage of recent
  enhancements.

These changes should allow for better handling of cases like those
described in python-poetry#3041. Additionally, this forms the foundation for
unifying HTTP specific logic within the code base and possibly allowing
for migration of requests etc. if/when required.
@abn @Darsstar
authenticator: allow multiple repos w/ same netloc
4a13cf2 
Co-authored-by: Agni Sairent <agniczech@gmail.com>
Co-authored-by: Dos Moonen <darsstar@gmail.com>
@neersighted neersighted merged commit 58db424 into python-poetry:master May 7, 2022
@abn abn deleted the http-repo-cleanup branch May 7, 2022 16:44
@abn abn mentioned this pull request May 8, 2022
Merged
This was referenced May 12, 2022
Closed
Closed
This was referenced May 20, 2022
Closed
Closed
@neersighted neersighted mentioned this pull request Jun 4, 2022
Closed
1 task
@abn abn mentioned this pull request Jun 6, 2022
Merged
@dimbleby dimbleby mentioned this pull request Jun 21, 2022
Merged
@FlotterCodername FlotterCodername mentioned this pull request Jun 23, 2022
Closed
3 tasks
@github-actions GitHub Actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@neersighted neersighted neersighted approved these changes

@Darsstar Darsstar Darsstar left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

403 error when using private myget repository
5 participants
@abn @Darsstar @agni-sairent @KundaPanda @neersighted